Skip to content

Don't require a normal tool build of clippy/rustfmt when running their test steps #149623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Don't require a normal tool build of clippy/rustfmt when running their test steps #149623

wants to merge 1 commit into from
+3 −6

Conversation

@jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Dec 4, 2025

This is redundant and wasteful.

Noticed in #147372 (comment).

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Dec 4, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2025

r? @Zalathar

rustbot has assigned @Zalathar.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@jieyouxu jieyouxu changed the title Avoid building the tool::$TOOL steps in non-test mode Avoid building the tool::$TOOL steps in non-test mode in test steps Dec 4, 2025
@jieyouxu jieyouxu mentioned this pull request Dec 4, 2025
Open
@Zalathar

This comment was marked as resolved.

Sign in to view
@jieyouxu
Copy link
Member Author

jieyouxu commented Dec 4, 2025

Sure. I agree the current wording is confusing, let me reword it.

@jieyouxu jieyouxu changed the title Avoid building the tool::$TOOL steps in non-test mode in test steps Don't require a normal tool build of clippy/rustfmt when running their test steps Dec 4, 2025
@jieyouxu
Don't require a normal tool build of clippy/rustfmt when running thei…
4d21e76 
…r test steps

This is redundant and wasteful.
@jieyouxu jieyouxu force-pushed the redundant-tool-test-step branch from 6413c4c to 4d21e76 Compare December 4, 2025 01:59
@jieyouxu
Copy link
Member Author

jieyouxu commented Dec 4, 2025

Reworded PR title/commit message as suggested.

@Zalathar
Copy link
Member

Zalathar commented Dec 4, 2025

Historical note: For rustfmt, this redundancy seems to date all the way back to the introduction of its test step, which at that time was a “check” step:

So at the time it was probably motivated by wanting to “test” as much as possible in CI in one step. Nowadays we would prefer the test step to only invoke cargo test; if building the tool normally fails then some other step will catch that.

@Zalathar
Copy link
Member

Zalathar commented Dec 4, 2025

I'm not very familiar with compiler staging of these tools, so hopefully you've done your homework. 😅

Let's also consult a try job that runs tests in stage 1, to augment PR CI:

@bors try jobs=x86_64-gnu-llvm-20-3

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors bot added a commit that referenced this pull request Dec 4, 2025
@rust-bors
Auto merge of #149623 - jieyouxu:redundant-tool-test-step, r=<try>
981a0d1 
Don't require a normal tool build of clippy/rustfmt when running their test steps

try-job: x86_64-gnu-llvm-20-3
@jieyouxu
Copy link
Member Author

jieyouxu commented Dec 4, 2025 •
edited
Loading

I'm not very familiar with compiler staging of these tools, so hopefully you've done your homework.

These should still be right, because they (rustfmt/clippy both being rustc_private tools) now use the RustcPrivateCompilers helper type which captures the {build_compiler,target_compiler} distinctions (where 2 compilers are involved!)

rust/src/bootstrap/src/core/build_steps/tool.rs

Lines 1324 to 1342 in 83e49b7

/// Represents which compilers are involved in the compilation of a tool
/// that depends on compiler internals (`rustc_private`).
/// Their compilation looks like this:
///
/// - `build_compiler` (stage N-1) builds `target_compiler` (stage N) to produce .rlibs
/// - These .rlibs are copied into the sysroot of `build_compiler`
/// - `build_compiler` (stage N-1) builds `<tool>` (stage N)
/// - `<tool>` links to .rlibs from `target_compiler`
///
/// Eventually, this could also be used for .rmetas and check builds, but so far we only deal with
/// normal builds here.
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
pub struct RustcPrivateCompilers {
/// Compiler that builds the tool and that builds `target_compiler`.
build_compiler: Compiler,
/// Compiler to which .rlib artifacts the tool links to.
/// The host target of this compiler corresponds to the target of the tool.
target_compiler: Compiler,
}

@Zalathar
Copy link
Member

Zalathar commented Dec 4, 2025

r=me when PR+try is green.

@rust-bors
Copy link

rust-bors bot commented Dec 4, 2025

☀️ Try build successful (CI)
Build commit: 981a0d1 (981a0d1725813fff9e47ad07e66b18ceaec26af5, parent: 83e49b75e7daf827e4390ae0ccbcb0d0e2c96493)

@jieyouxu
Copy link
Member Author

jieyouxu commented Dec 4, 2025

Both PR+try are green.

@bors r=Zalathar rollup

1 similar comment
@jieyouxu
Copy link
Member Author

jieyouxu commented Dec 4, 2025

Both PR+try are green.

@bors r=Zalathar rollup

@bors
Copy link
Collaborator

bors commented Dec 4, 2025

📌 Commit 4d21e76 has been approved by Zalathar

It is now in the queue for this repository.

1 similar comment
@bors
Copy link
Collaborator

bors commented Dec 4, 2025

📌 Commit 4d21e76 has been approved by Zalathar

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 4, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 4, 2025
@matthiaskrgr
Rollup merge of rust-lang#149623 - jieyouxu:redundant-tool-test-step,…
f2d4955 
… r=Zalathar

Don't require a normal tool build of clippy/rustfmt when running their test steps

This is redundant and wasteful.

Noticed in rust-lang#147372 (comment).
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 4, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Zalathar Zalathar

Labels

A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jieyouxu @rustbot @Zalathar @bors